-
Notifications
You must be signed in to change notification settings - Fork 289
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
blockchain: Add UtxoBackend interface. #2652
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've reviewed about half of this so far. The direction looks like what we discussed and I really appreciate the separation of the commits to make the review easier, especially for larger changes such as these.
I figured I'd go ahead and post the few things I've spotted so far while I review the remaining commits.
} | ||
key := outpointKey(outpoint) | ||
err = utxoBucket.Put(*key, serialized) | ||
// NOTE: The key is intentionally not recycled here since the database |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realize that as of this commit and PR, it's still actually using a database.DB
behind the scenes, so this has to remain and is definitely true for now, but I figured I'd point it out as something to keep in mind when replacing the backend as I would expect this to no longer necessarily be the case for a new backend that is designed specifically for the utxo set that doesn't go through the database.DB
interface.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, agreed. That is a good call-out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've finished reviewing everything. It looks great overall. Once the review items are addressed, I'll put it through the paces.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've reviewed all of the updates. I'll be testing this over the next few days, but I don't expect any issues based on the review.
This removes the dependency that the UtxoCacher interface and UtxoCache implementation currently have on the database.Tx interface. This is necessary since the UtxoCache will move away from the current database interface in favor of an interface that is better suited for the UTXO database in a subsequent commit.
This adds a UtxoBackend interface and LevelDbUtxoBackend type, which implements that interface. This UtxoBackend interface will allow different backend implementations to be used by the UtxoCache rather than interacting directly with the database layer as it does currently. In subsequent commits, interface methods and the corresponding implementation will be added to UtxoBackend one at a time in order to ease the review process as things move around.
This exports the UtxoSetState type since it will be used in the UtxoBackend interface in a subsequent commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great!
This adds the FetchEntry method to the UtxoBackend interface and LevelDbUtxoBackend implementation. A summary of the changes is as follows: - Add FetchEntry method to UtxoBackend interface - Add FetchEntry method to LevelDbUtxoBackend - Move dbFetchUtxoEntry to LevelDbUtxoBackend - Add backend tests for FetchEntry - Update UtxoCache to fetch entries from the backend rather than from the database directly
This adds the PutUtxos method to the UtxoBackend interface and LevelDbUtxoBackend implementation. A summary of the changes is as follows: - Add PutUtxos method to UtxoBackend interface, which atomically updates the UTXO set and state - Add PutUtxos method to LevelDbUtxoBackend - Move dbPutUtxoEntry to LevelDbUtxoBackend - Add backend tests for PutUtxos - Update UtxoCache to use PutUtxos rather than updating the database directly
This adds the FetchState method to the UtxoBackend interface and LevelDbUtxoBackend implementation. A summary of the changes is as follows: - Add FetchState method to UtxoBackend interface - Add FetchState method to LevelDbUtxoBackend - Add backend tests for FetchState - Update UtxoCache to use FetchState rather than querying the database directly - Remove dbPutUtxoSetState and dbFetchUtxoSetState since they are no longer used - Add FetchBackendState to the UtxoCache so that the state can be fetched through the cache
This adds the FetchStats method to the UtxoBackend interface and LevelDbUtxoBackend implementation. A summary of the changes is as follows: - Add FetchStats method to UtxoBackend interface - Add FetchStats method to LevelDbUtxoBackend - Move dbFetchUtxoStats to LevelDbUtxoBackend - Add FetchStats to the UtxoCache so that the stats can be fetched through the cache
This adds the FetchInfo, InitInfo, and PutInfo methods to the UtxoBackend interface and LevelDbUtxoBackend implementation. A summary of the changes is as follows: - Add FetchInfo, InitInfo, and PutInfo methods to UtxoBackend interface - Add FetchInfo, InitInfo, and PutInfo methods to LevelDbUtxoBackend - Move dbPutUtxoDatabaseInfo, dbFetchUtxoDatabaseInfo, and associated variables to LevelDbUtxoBackend - Move initUtxoDbInfo, createUtxoDbInfo, and associated variables to LevelDbUtxoBackend - Rename and export utxoDatabaseInfo to UtxoBackendInfo - Add backend tests for putting and fetching the UTXO backend info - Add utxoBackend to BlockChain Config since BlockChain handles initializing the UTXO backend - Remove utxoDbInfo from BlockChain and instead access the UTXO backend info through the UTXO backend
This moves LoadUtxoDB and the associated helper functions and variables from utxodb.go to utxobackend.go since LoadUtxoDB is specific to the LevelDbUtxoBackend implementation of the UtxoBackend interface.
This adds the Upgrade method to the UtxoBackend interface and LevelDbUtxoBackend implementation. A summary of the changes is as follows: - Add Upgrade method to UtxoBackend interface - Add Upgrade method to LevelDbUtxoBackend - Remove initUtxoState and instead handle upgrading through the UtxoCache Initialize method - Pass the UTXO backend into upgradeUtxoDb rather than the entire BlockChain instance
This removes the UTXO db from the BlockChain and UtxoCache types. BlockChain now only accesses the UTXO db through the UtxoCache and the UtxoCache now only accesses the UTXO db through the UtxoBackend.
This exports the ViewFilteredSet type since it is used in the UtxoCacher interface.
This reworks the UTXO related logic to use a
UtxoBackend
interface. It is broken up into several commits to ease the review process as things moved around.High-level design (
BlockChain
/UtxoViewpoint
->UtxoCacher
->UtxoBackend
)BlockChain
andUtxoViewpoint
interact with theUtxoCacher
interface and do not directly interact withUtxoBackend
or the underlying databaseUtxoCache
interacts with theUtxoBackend
interface and does not directly interact with the underlying databaseUtxoBackend
encapsulates the logic that is specific to the backend implementation being usedutxoio.go
now only contains database-agnostic serialization functions, and anything specific to the underlying database was moved toutxobackend.go
Motivation
LevelDbUtxoBackend
implementation in terms of memory usage and processing time (with the initial approach being to useleveldb
directly instead of the existingdatabase.DB
interface). The changes required for that can now be localized toLevelDbUtxoBackend
without having to change much elseAdditional Notes
upgrade.go
still deals with the UTXO database directlyCursor
andTx
interfaces and associated methods toUtxoBackend
in this PR to allowUtxoBackend
to be used here rather than the UTXO database directlydatabase.DB
interface is currently doing, which we will be moving away from shortly. So it will make more sense to add this functionality to the interface after moving away from thedatabase.DB
interfaceUtxoBackend
interface method to query the existence of a specific UTXO without retrieving it would be useful